Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shapes plugin #107

Merged
merged 21 commits into from
May 14, 2020
Merged

Add shapes plugin #107

merged 21 commits into from
May 14, 2020

Conversation

JShep1
Copy link

@JShep1 JShep1 commented Apr 28, 2020

This PR makes use of ign-gazebo#95 so that PR will need to be merged before this one (it doesn't have to be, but I thought a separate PR for that functionality would be better for visibility and documentation purposes.

There are a few things I made "prototypes" of that I would like to have looked and discussed for improvement:

  • The unused entity id generation in UniqueId()
  • How should the hovered mouse position be stored for best scalability and access for future users of the value
  • Should the code for creating a model from the sdf string be moved to a separate function?
  • Is the current location of HandleModelPlacement() good/could there be a better way to handle the mouse events here?

There's currently a bug with this addition messing with the button pressing functionalities - I'll take a closer look at this soon

Resolves #81

@JShep1
Copy link
Author

JShep1 commented Apr 28, 2020

Demo:
shapesplugin

@JShep1 JShep1 requested review from chapulina and iche033 April 28, 2020 07:23
@chapulina chapulina added the 📜 blueprint Ignition Blueprint label Apr 28, 2020
Signed-off-by: John Shepherd <[email protected]>
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's working for me!

Should the code for creating a model from the sdf string be moved to a separate function?

+1


I noticed this issue when spawning entities from below the ground. The spawned pose is slightly offset from the preview pose at click time:

place_change_pos

src/gui/plugins/scene3d/Scene3D.cc Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.hh Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
This was referenced Apr 29, 2020
@JShep1
Copy link
Author

JShep1 commented May 5, 2020

I noticed this issue when spawning entities from below the ground. The spawned pose is slightly offset from the preview pose at click time:

Fixed in dd864dd

@JShep1 JShep1 requested a review from chapulina May 6, 2020 06:54
@JShep1
Copy link
Author

JShep1 commented May 6, 2020

Ready for another iteration

include/ignition/gazebo/rendering/SceneManager.hh Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
src/gui/plugins/shapes/Shapes.qml Outdated Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
Signed-off-by: John Shepherd <[email protected]>
@codecov
Copy link

codecov bot commented May 10, 2020

Codecov Report

❗ No coverage uploaded for pull request base (ign-gazebo2@dd5c72b). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             ign-gazebo2     #107   +/-   ##
==============================================
  Coverage               ?   62.38%           
==============================================
  Files                  ?      123           
  Lines                  ?     6091           
  Branches               ?        0           
==============================================
  Hits                   ?     3800           
  Misses                 ?     2291           
  Partials               ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd5c72b...5d7f4ac. Read the comment docs.

John Shepherd added 2 commits May 12, 2020 13:32
Signed-off-by: John Shepherd <[email protected]>
Signed-off-by: John Shepherd <[email protected]>
@JShep1 JShep1 requested a review from iche033 May 12, 2020 20:39
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new mouse hover events works great

I only have a couple comments left. Otherwise looks good to me.

src/gui/plugins/scene3d/Scene3D.hh Show resolved Hide resolved
src/gui/plugins/scene3d/Scene3D.cc Outdated Show resolved Hide resolved
John Shepherd added 2 commits May 12, 2020 15:30
Signed-off-by: John Shepherd <[email protected]>
Signed-off-by: John Shepherd <[email protected]>
@JShep1 JShep1 requested a review from iche033 May 13, 2020 17:12
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great!

@JShep1 JShep1 merged commit 24573da into ign-gazebo2 May 14, 2020
@JShep1 JShep1 deleted the add_shapes_plugin branch May 14, 2020 17:19
azeey pushed a commit to azeey/gz-sim that referenced this pull request May 20, 2020
nkoenig added a commit that referenced this pull request May 21, 2020
* Add shapes plugin (#107)

Signed-off-by: John Shepherd <[email protected]>

* Add option to suppress warning about missing child model (#132)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Support multi entity spawn (#146)

* Support multi entity spawn

Signed-off-by: Nate Koenig <[email protected]>

* Added more documentation

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: Nate Koenig <[email protected]>

* Bump ign-msgs version to 4.9 (#148)

Signed-off-by: Louise Poubel <[email protected]>

* Revert silly mistake

Signed-off-by: Nate Koenig <[email protected]>

* ign-msgs 5.3 dependency

Signed-off-by: Nate Koenig <[email protected]>

* Bump minor version

Signed-off-by: Nate Koenig <[email protected]>

* Changelog

Signed-off-by: Nate Koenig <[email protected]>

* RawPose

Signed-off-by: Nate Koenig <[email protected]>

Co-authored-by: John <[email protected]>
Co-authored-by: Addisu Taddese <[email protected]>
Co-authored-by: Nate Koenig <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
azeey pushed a commit to azeey/gz-sim that referenced this pull request May 28, 2020
Signed-off-by: John Shepherd <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants